Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic shallow rendering support #2497

Merged
merged 1 commit into from
Nov 17, 2014
Merged

Basic shallow rendering support #2497

merged 1 commit into from
Nov 17, 2014

Conversation

graue
Copy link
Contributor

@graue graue commented Nov 11, 2014

WIP to fix #2393.

Now redone on top of the new ReactCompositeComponent/ReactClass separation (#2445). Still only handles initial rendering not updating, but this is a simpler implementation that should be easier to add the missing parts to.

Sorry about closing the old PR (#2459), I'll just force-push next time I do a new version of a PR.

As a bonus, per @sebmarkbage's recommendation, this defines the validated flag to be a non-enumerable property, and avoids setting owner on the shallowly-rendered component, so we can now do a straight equality test on the expected and actual results and avoid helper functions like areElementsEquivalent.

assign(
ShallowCompositeComponentWrapper.prototype,
ReactCompositeComponent.ShallowMixin
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not really using any logic in this file. It's mostly just validation logic and figuring out what to do about native components. Since you won't test native components using this strategy, you won't shallow render native components.

Once you remove all the stuff you're not using, this will simply just be constructing a new instance of the ShallowMixin. You can probably just move all that stuff into the ReactTestUtils so we don't have to deal with this intermediate.

@graue
Copy link
Contributor Author

graue commented Nov 11, 2014

Good point about not needing instantiateReactComponent for shallow rendering. Pushed a small incremental commit to clean that up.

@graue
Copy link
Contributor Author

graue commented Nov 13, 2014

Had a little more time to work on this and got updating working. Also managed to simplify the code somewhat. I went back to using transactions since it was adding more duplication/complexity not to and they don't seem to hurt anything. Interested in any thoughts, @sebmarkbage.

@sebmarkbage
Copy link
Collaborator

This looks good but this is probably going to conflict with some of the other PRs. So let's wait to rebase on top of them so we don't have to thrash them so much.

@graue
Copy link
Contributor Author

graue commented Nov 17, 2014

That sounds fine. Any PRs in particular to look out for?

@sebmarkbage
Copy link
Collaborator

I think they're all in now so please rebase and land asap to avoid more conflicts.

Now handles updating. Haven't looked at refs yet.
graue added a commit that referenced this pull request Nov 17, 2014
Basic shallow rendering support (#2393)
@graue graue merged commit d75512f into facebook:master Nov 17, 2014
@graue graue deleted the 2393-v1 branch January 27, 2015 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Shallow Testing
2 participants